Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reapply "Load analyzers and generators in isolated ALCs in our OOP process" #75233

Merged
merged 11 commits into from
Oct 7, 2024

Conversation

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner September 25, 2024 15:01
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Sep 25, 2024
@jaredpar
Copy link
Member

Can we get an ETW counter for every time we tear down / rebuild the assembly loader? That is one item we felt would've helped the initial investigation also seems like something we could get David to add as a critical counter for VS insertions. That would block other teams from introducing changes that caused us to reset load context too often.

@CyrusNajmabadi
Copy link
Member Author

Can we get an ETW counter for every time we tear down / rebuild the assembly loader?

We can add counters for this.

@CyrusNajmabadi
Copy link
Member Author

draft while i wait on speedometer

@CyrusNajmabadi
Copy link
Member Author

@ToddGrun @jasonmalinowski this is ready for review. You just need to review the last two commits.

@@ -427,6 +427,8 @@ public bool TryFetch(LocalUserRegistryOptionPersister persister, OptionKey2 opti
{"dotnet_enable_diagnostics_in_source_generated_files_feature_flag", new FeatureFlagStorage(@"Roslyn.EnableDiagnosticsInSourceGeneratedFiles")},
{"dotnet_source_generator_execution", new RoamingProfileStorage("TextEditor.Roslyn.Specific.SourceGeneratorExecution")},
{"dotnet_source_generator_execution_balanced_feature_flag", new FeatureFlagStorage(@"Roslyn.SourceGeneratorExecutionBalanced")},
{"dotnet_reload_changed_analyzer_references", new RoamingProfileStorage("TextEditor.Roslyn.Specific.ReloadChangedAnalyzerReferences")},
{"dotnet_reload_changed_analyzer_references_feature_flag", new FeatureFlagStorage(@"Roslyn.ReloadChangedAnalyzerReferences")},
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these should be part of the next commit. oh well.

@333fred
Copy link
Member

333fred commented Oct 1, 2024

I'd prefer to hold on merging this until we get the assembly-binding things fixed up in main, as that appears to be causing ngen failures and jitting regressions. I'm not thinking this PR is responsible, but I just want to keep large variables out of the equation.

@@ -26,36 +26,36 @@
<value>[base64 mime encoded string representing a byte array form of the .NET Framework object]</value>
<comment>This is a comment</comment>
</data>

There are any number of "resheader" rows that contain simple

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's up with the whitespace changes in this file?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Constantly happens when I use the vs resx editor. No clue why.

Copy link
Contributor

@ToddGrun ToddGrun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@@ -272,6 +278,16 @@ private void DisplayInlineTypeHints_Unchecked(object sender, RoutedEventArgs e)
UpdateInlineHintsOptions();
}

private void RunCodeAnalysisInSeparateProcess_Checked(object sender, RoutedEventArgs e)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this fire only if the user changes it or if it's changed programmatically? The useful test would be to turn it off, restart VS, and see if we disable it when the options page loads the first time.

@CyrusNajmabadi
Copy link
Member Author

@jasonmalinowski ptal.

@CyrusNajmabadi
Copy link
Member Author

Got confirmation from infra that this can go in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Needs UX Triage untriaged Issues and PRs which have not yet been triaged by a lead VSCode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants